Copy all selected thoughts when Select All (multicursor) is active#4438
Conversation
Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
BayuAri
left a comment
There was a problem hiding this comment.
Issue is partially fixed.
It works on Desktop Chrome but there is an issue on Desktop Safari.
Importing from clpboard... alert is flashed after Cmd + V but the copied thoughts are not pasted as it is on Chrome
@copilot
Please fix this issue on Desktop Safari
… copy (#3993) Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
Fixed in 41ee407. The root cause was that the structured clipboard formats ( The device-level |
Safari only honors clipboard setData() in a genuine user-initiated copy event, and dispatches the Cmd+C copy event on a later task than setTimeout(0). With a multicursor no editable is focused, so the copy event targets <body> and useOnCopy never runs; the browser then serializes the empty collapsed selection and the previous async write clobbered it, leaving an empty clipboard. Register a capture-phase document copy listener that intercepts the late user-initiated copy event regardless of focus target, preventDefault()s the empty serialization, and writes text/plain + text/html + text/em via setData (honored by Safari). The listener self-removes on the copy event and is cleaned up after a generous window otherwise. Non-Safari browsers keep the proven execCommand+setData path used by the e2e suite. Add a selectNode() helper used by the non-Safari rich copy path.
Safari multicursor copy/paste fix (#3993)ProblemThe original fix resolved multicursor copy/paste on desktop Chrome, but paste was still broken on Safari — pasting produced nothing, and the clipboard contained only an empty Root cause (established empirically on real Safari)Safari's clipboard behaves very differently from Chrome:
SolutionOn Safari,
The listener self-removes on the copy event and is cleaned up after a generous window otherwise (so it can't affect an unrelated later copy). Non-Safari browsers (Chrome + the Puppeteer e2e suite) keep the proven VerificationTested live on Safari across many attempts — copy/paste is now consistent: every paste arrives with Known separate issue (not addressed here)When a copied selection contains an empty thought ( |
|
Test Result
@copilot Step to Reproduce
Current behaviorNothing happens Expected behaviorCopied thoughts should be pasted |
…3993) Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
Fixed in 4b3d61b. Root cause on Mobile Safari: the rich copy path ( Mobile Safari now routes to the plain-text ClipboardJS Note: on Mobile Safari this preserves the full thought structure but not inline formatting (bold/italic/color), since that requires |
BayuAri
left a comment
There was a problem hiding this comment.
Test Result
- Desktop Chrome
- Desktop Safari
- Mobile Chrome
- Mobile Safari
Confirm all platforms are working as expected.
raineorshine
left a comment
There was a problem hiding this comment.
This touches a lot of the copy infrastructure which may affect other cases. Before I review, could you test the following:
- Copy thoughts with subthoughts.
- Copy thoughts and subthoughts with different colors.
These are the cases where a regression is most likely to occur.
Note that copying colored thoughts is already a bit broken on main, so if you encounter one of those cases you can just note it and I will make a judgment call about whether I think it is fully independent of this fix.
Thanks!
@raineorshine I have a question about how the Copied alert modal when the Selected modal is on, can you help me with the expected result of this behavior, please? Suggestion:
The expected result may relate to #3612 Currently, the Copied alert flashes in milli second
Flashing.Copied.Alert.when.Selected.Alert.is.onscreen.mov |
Thanks!
Actually the new Liminal UI for multiselect won't have this conflict, because the multicursor selection state will no longer be shown in an alert. So we can leave the broken alert behavior for now. |
| // When a multicursor is active, copy all selected thoughts, not just the focused one. | ||
| // The Copy Cursor command also writes the full selection to the clipboard, but copyCursor uses | ||
| // permitDefault, so this native copy event fires afterward and would otherwise clobber it with | ||
| // only the focused thought (#3993). | ||
| if (hasMulticursor(state)) { | ||
| const ids = getMulticursorThoughtIds(state) | ||
| const plainText = trimBullet(ids.map(id => strip(exportContext(state, id, 'text/plain'))).join('\n')) | ||
| const htmlText = ids.map(id => exportContext(state, id, 'text/html')).join('\n') | ||
| clipboardData.setData('text/plain', plainText) | ||
| clipboardData.setData('text/html', htmlText) | ||
| clipboardData.setData('text/em', 'true') | ||
| return | ||
| } | ||
|
|
||
| const currentText = selection.text() | ||
| const currentHtml = selection.html() | ||
| const clipboardData = event.clipboardData | ||
| clipboardData.setData('text/plain', currentText!) | ||
| clipboardData.setData('text/html', currentHtml!) | ||
|
|
||
| if (!currentText) { | ||
| const thoughtHtml = exportContext(state, thoughtId, 'text/html') | ||
| const thoughtText = exportContext(state, thoughtId, 'text/plain') | ||
| clipboardData.setData('text/plain', trimBullet(thoughtText)) | ||
| clipboardData.setData('text/html', thoughtHtml) | ||
| } | ||
| clipboardData.setData('text/em', 'true') |
There was a problem hiding this comment.
There is a lot of duplication between these different blocks. Instead of doing this with separate blocks with multiple return statements, funnel all the logic into a single return statement at the end. Use ternary expressions to reduce conditional boilerplate.
There was a problem hiding this comment.
Refactored: the handler now computes plainText and htmlText via ternaries (multicursor → all selected thoughts; else current selection, falling back to the focused thought when empty) and writes them in a single set of setData calls, removing the separate blocks and early return.
| // onCopy self-removes on the first copy event (setting done); this timeout only removes the listener if no | ||
| // copy event ever arrives (e.g. the command was triggered from the command palette) so it cannot affect an | ||
| // unrelated later copy. | ||
| setTimeout(() => { |
There was a problem hiding this comment.
Does this handle the case where copyRichSafari is called multiple times within the 1s window? I don't see anything that cancels the timeout.
There was a problem hiding this comment.
Good catch — it didn't. copyRichSafari now tracks the pending listener + timeout in a module-level pendingSafariCopy and calls clearPendingSafariCopy() at the start of each invocation, so a second call within the 1s window cancels the prior listener and its timeout before registering the new one. The timeout and the copy handler both route through clearPendingSafariCopy.
|
@copilot Please address the review feedback. |
…afari copy timeout (#3993) Co-authored-by: raineorshine <750276+raineorshine@users.noreply.github.com>
|
|

On desktop, copying with Select All active only copied the focused thought even though the alert reported the full count ("Copied 3 thoughts"), so pasting yielded a single thought instead of the whole selection.
Root cause
copyCursorruns withpermitDefault: true, sopreventDefault()is never called. After the command writes the full multicursor selection to the clipboard, the browser's nativecopyevent still fires on the focused editable, invokinguseOnCopy, which exported only the single focused thought — overwriting the clipboard. This is desktop-only because headless Chrome does not emit acopyevent for a collapsed contenteditable selection.On Desktop Safari the same fix was insufficient: the structured clipboard formats (
text/htmland thetext/emmarker) were only written by the nativecopyevent handler, but Safari (like headless Chrome) does not fire acopyevent for a collapsed contenteditable selection. As a result only plain text reached the clipboard, and on paste Safari synthesized its own generictext/htmlthat shadowed the clean plain text inimportData— so the "Importing from clipboard..." alert flashed but nothing was pasted.On Mobile Safari the rich-copy path could not run at all: it depends on a genuine, user-initiated Cmd+C
copyevent, but the copy is triggered by a Command Center tap (no keyboard), so no nativecopyevent ever fires, and Safari ignoressetData()during a programmatic copy. The rich-copy listener therefore waited, timed out, and wrote nothing — so nothing pasted.Changes
src/selectors/getMulticursorThoughtIds.ts(new): returns the multicursor selection as document-orderedThoughtId[], deduping descendants when an ancestor is also selected.src/components/Editable/useOnCopy.ts: whenhasMulticursor(state), export all selected thoughts (text/plain,text/html,text/em) and return early; single-thought path is unchanged.src/commands/copyCursor.ts: reusegetMulticursorThoughtIdsinexecMulticursorin place of the inline dedup; pass the exported html tocopy().src/device/copy.ts:copy(text, { html })now writestext/plain+text/html+text/emdeterministically via a one-shot documentcopylistener (triggered through ClipboardJS'sexecCommand('copy')), independent of the native editable copy event. This makes structured paste work on browsers that do not fire a native copy event for a collapsed selection (Desktop Safari, headless Chrome). The native Capacitor and plain-only paths are unchanged, and selection is still saved/restored. On mobile Safari (isSafari() && isTouch),copy()skips the rich path and uses the plain-text ClipboardJS path, which runs synchronously within the tap gesture: the full multicursor selection is still copied as indented plain text that em reconstructs into the thought tree on paste (thought structure is preserved; inline formatting is not, sincetext/htmlcannot be written without a real copy event on iOS).src/commands/__tests__/copyCursor.ts: updated assertions for the newcopy()call signature.src/e2e/puppeteer/__tests__/multiselect.ts: regression tests — one dispatching a syntheticcopyevent with multicursor active asserting the clipboard contains all selected thoughts, and one interceptingDataTransfer.setDatato assert the copy command writestext/html+text/emfor the full selection (verified to fail without the device-level fix).